Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Deploying drive-web with
|
| Latest commit: |
6a35e91
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://ff7a4d84.drive-web.pages.dev |
| Branch Preview URL: | https://opaque-poc.drive-web.pages.dev |
| import { AuthMethodTypes } from 'app/payment/types'; | ||
| import vpnAuthService from 'app/auth/services/vpnAuth.service'; | ||
| import envService from 'app/core/services/env.service'; | ||
| import { logInOpaque, is2FAorOpaqueNeeded } from '../../services/auth.opaque'; |
There was a problem hiding this comment.
This should be in the auth service, let's not create 'helper' files
|
|
||
| try { | ||
| const isTfaEnabled = await is2FANeeded(email); | ||
| const { tfaEnabled: isTfaEnabled, opaqueLogin } = await is2FAorOpaqueNeeded(email); |
There was a problem hiding this comment.
Could we hide these things under the same functions? Otherwise we mix component-related logic with authentication low-level details, which is far from ideal in terms of maintenance and testability. Let's hide these things under the auth layer.
| }; | ||
|
|
||
| const { token, user, mnemonic } = await authenticateUser(authParams); | ||
| const { token, user, mnemonic } = await (opaqueLogin ? logInOpaque(authParams) : authenticateUser(authParams)); |
| import { generateUserSecrets, encryptUserKeysAndMnemonic, decryptUserKeysAndMnemonic } from './auth.crypto'; | ||
|
|
||
| describe('Test auth crypto functions', () => { | ||
| it('test enc/dec of user sercrets', async () => { |
There was a problem hiding this comment.
Use When-then legends and define clearly what each case is doing / testing
|
|
||
| const { encMnemonic, encKeys } = await encryptUserKeysAndMnemonic(keys, mnemonic, exportKey); | ||
|
|
||
| const { keys: dec_keys, mnemonic: dec_mnemonic } = await decryptUserKeysAndMnemonic( |
There was a problem hiding this comment.
Avoid using snake_case, the convention is camelCase
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
larryrider
left a comment
larryrider
left a comment
There was a problem hiding this comment.
check the build action, it seems to be failing right now
|
@larryrider, hash library is not working with node less than 20, so tests on 18 fail due to failed build |
@TamaraFinogina We should then update those actions to use node 20 or 22 |
src/services/auth.opaque.ts
Outdated
| password: string, | ||
| twoFactorCode: string, | ||
| ): Promise<{ token: string; user: UserSettings; mnemonic: string; newToken: string }> => { | ||
| const { sessionID, user: encUser, sessionKey, exportKey } = await loginOpaque(email, password, twoFactorCode); |
There was a problem hiding this comment.
Not sure if calling that encUser is the best option since since the user isn't actually encrypted. What about loggedUser or smth like that?
src/app/auth/services/auth.opaque.ts
Outdated
| localStorageService.set('xNewToken', sessionID); | ||
| await setSessionKey(password, sessionKey); | ||
|
|
||
| Sentry.setUser({ |
There was a problem hiding this comment.
I think Sentry can be safely removed. We do not use it anymore.
src/services/auth.opaque.ts
Outdated
| localStorageService.set('sessionKey', sessionKeyEnc); | ||
| localStorageService.set('sessionKeySalt', salt); | ||
| }; | ||
|
|
||
| export const getSessionKey = async (password: string): Promise<Uint8Array> => { | ||
| const sessionKeyEnc = localStorageService.get('sessionKey') || ''; | ||
| const salt = localStorageService.get('sessionKeySalt') || ''; |
There was a problem hiding this comment.
Perhaps you could create a function in localStorageService called setSessionKey and getSessionKey and manage there the keys/values instead? WDYT?
src/services/auth.opaque.ts
Outdated
| password, | ||
| }); | ||
| if (!loginResult) { | ||
| throw new Error('Login failed'); |
There was a problem hiding this comment.
Suggestion: Consider creating a custom error (e.g., LoginFailedError) so it’s clear that the error originates from our code. This makes debugging easier. You can use something like this as a reference.
|
5 participants
Description
This PR is for the PoC for opaque login (just to agree on the API).
During login, we call
is2FAorOpaqueNeeded, which tells us if login should be with Opaque or not. The draft implementation for opaque auth is insrc/app/auth/services/auth.opaque.ts.Related Issues
Proof of Concept for PB-4610
Related Pull Requests
Checklist
Testing Process
No tests are added yet.
Additional Notes
To build, you need to link the SDK from the opaque PoC PR.